Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Document differences per shell #3207

Merged
merged 10 commits into from
Aug 31, 2023
Merged

Document differences per shell #3207

merged 10 commits into from
Aug 31, 2023

Conversation

theunrepentantgeek
Copy link
Member

What this PR does / why we need it:

We've had users who encountered problems because they were using a different shell (not bash) and got tripped up by subtle differences in command-line syntax.

For example, bash requires 'single quotes' around crdPattern to avoid expansion of any * wildcards, but if you include those same 'quotes' when using cmd you end up with patterns that don't work.

This PR splits our landing pages into different forms for GitHub vs our documentation site.

For GitHub, we include a warning about the examples being bash specific. For our documentation site, we show examples for each shell.

Closes #3145 by showing the different syntax for each shell on our documentation site.

Closes #3142 by showing how crdPattern works different for each shell

How does this PR make you feel:
gif

If applicable:

  • this PR contains documentation

@theunrepentantgeek theunrepentantgeek self-assigned this Aug 25, 2023
@theunrepentantgeek theunrepentantgeek added the documentation Improvements or additions to documentation label Aug 25, 2023
@matthchr matthchr added this to the v2.3.0 milestone Aug 25, 2023
Copy link
Member

@matthchr matthchr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sold that having the ability to select shell for the options that are exactly the same is worth it.

For example, for

$ kubectl delete resourcegroups/aso-sample-rg

modulo the starting character, this command is identical on all shells. So I feel like we don't need to give users shell-selection for it? offering shell selection on these options adds a lot of boilerplate to this file (in terms of the markdown), and also adds a lot of clickable options that IMO end up distracting from the simplicity of the installation process in the rendered UI.

Could we consider one of the following options:

  1. Offer the shell selection only once, and then give all of the rest of the instructions inside of that selection? So the user just chooses their shell once at the top and is done, rather than having to pick it N times? This would reduce the amount of screen real-estate devoted to the shell choices and simplify the user options (pick once), though it might mean even more duplication in the actual markdown.
  2. Only offer shell selection where the cmd actually materially differs? (This would remove by my count ~8 or 9 selectors)

Note: I can't actually leave my comment on the _index.md file for some reason, having the same issue as @super-harsh was.

v2/README.md Outdated

> [!WARNING]
> If you `kubectl delete` an Azure resource from your cluster, ASO ***will*** delete the matching Azure resource. This can
> be dangerous if you were to do this with an existing resource group which contains resources you do not wish to be deleted.**
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
> be dangerous if you were to do this with an existing resource group which contains resources you do not wish to be deleted.**
> be dangerous if you were to do this with an existing resource group which contains resources you do not wish to be deleted.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

@theunrepentantgeek
Copy link
Member Author

I've updated to remove tabs where the content is identical.

I suspect GH is having trouble with the transformation from a soft-link to an actual file, and this is why comments on the file aren't available.

@super-harsh
Copy link
Collaborator

On line 274, the events look old, would be good to update with how they look now currently

@theunrepentantgeek
Copy link
Member Author

On line 274, the events look old, would be good to update with how they look now currently

Done.

@theunrepentantgeek theunrepentantgeek enabled auto-merge (squash) August 31, 2023 01:34
@theunrepentantgeek theunrepentantgeek merged commit 0722dc4 into main Aug 31, 2023
@theunrepentantgeek theunrepentantgeek deleted the doc/shell-differences branch August 31, 2023 01:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
Development

Successfully merging this pull request may close these issues.

Improve documentation on shells Improve documentation for crdPattern
3 participants